Skip to content

greatly expand testsuite coverage and add coverage analysis with gcov#913

Merged
tridge merged 15 commits into
RsyncProject:masterfrom
tridge:pr-test-coverage
May 24, 2026
Merged

greatly expand testsuite coverage and add coverage analysis with gcov#913
tridge merged 15 commits into
RsyncProject:masterfrom
tridge:pr-test-coverage

Conversation

@tridge
Copy link
Copy Markdown
Member

@tridge tridge commented May 23, 2026

in preparation for some restructuring work this expands the testsuite a lot and add html coverage reports

tridge and others added 14 commits May 24, 2026 07:47
Add helpers for the option-coverage expansion (the path-handling restructure
changes parent-component resolution, so options must be exercised at depth and
across directory boundaries):

  * make_tree() builds a tree with a regular file at every level so a property
    can be checked at the tree root and >=3 levels deep;
  * walk_files()/walk_dirs() iterate entries for per-level assertions;
  * assert_same/assert_mode/assert_mtime_close/assert_is_symlink/
    assert_hardlinked/assert_not_hardlinked/assert_exists/assert_not_exists
    assert the concrete property an option controls (not just dest == src);
  * write_daemon_conf() writes an arbitrary rsyncd.conf (globals + modules)
    for daemon-parameter tests, beyond build_rsyncd_conf's fixed four modules;
  * forced_protocol() lets protocol-sensitive tests gate sub-cases.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fill the highest-restructure-risk gap: options that do two-directory / rename /
outside-tree work, asserted at >=3 levels deep with the aux tree kept outside
the main tree, and asserting the option's specific property rather than just
tree equality (which the ported tests already cover).

  alt-dest-deep  --link-dest hardlinks unchanged files (same inode), --copy-dest
                 copies (never links), --compare-dest omits unchanged files;
                 ref tree outside both src and dest.
  temp-dir       cross-dir temp->final rename at depth; temp dir left clean; a
                 missing --temp-dir fails (so the option is proven consulted).
  partial        --partial keeps the partial in the dest file; relative
                 --partial-dir stages per-directory at depth (pre-seed +
                 interrupt/resume); absolute --partial-dir writes the partial
                 outside the tree.
  inplace        --inplace keeps the destination inode across a delta update;
                 the default temp+rename path replaces it.
  append         --append completes truncated files tail-only; --append-verify
                 repairs a corrupted prefix (protocol >= 30).
  backup-deep    --suffix saves <name>S beside the new file; --backup-dir
                 relocates old files to a parallel deep tree outside the dest
                 and captures deletions under --delete.

All green on master and under --protocol=29/30.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cover the structure and link options at >=3 levels and across directories,
asserting each option's specific effect:

  links            -l keeps a symlink, -L dereferences it, -k follows a
                   directory symlink -- all on a symlink several levels deep.
  dirs             -d copies the top layer (file + empty dir) without recursing.
  prune-empty-dirs -m drops empty chains and chains emptied by an exclude,
                   keeps populated ones.
  hardlinks-deep   -H preserves a hard link whose names live in different
                   directories at depth; without -H they become separate inodes.
  delete-deep      --delete removes a deep extraneous file/subtree; the four
                   delete-timing variants agree; --max-delete caps deletions;
                   --existing / --ignore-existing select/skip correctly.
  relative-implied -R mirrors an implied directory's mode at depth;
                   --no-implied-dirs does not (proto 30+).

Green on master and under --protocol=29/30 (the --no-implied-dirs sub-case is
gated to protocol >= 30, where multi-component sender paths are accepted).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Set each attribute distinctively on a file AND a directory at every level of a
>=3-deep tree and verify it per entry after transfer (metadata is applied as a
single-component op on an entry whose parent chain the resolver restructure
rewrites):

  metadata-depth   -p preserves exact file/dir modes; -t preserves file
                   mtimes; --chmod=D710,F600 rewrites them.
  omit-times       -O omits directory times (files still preserved); -J omits
                   symlink times.
  sparse           -S preserves a deep file's hole (allocated << size);
                   --no-sparse fills it.
  xattrs-depth     -X reproduces a user xattr on every entry (gated on xattr
                   support).
  acls-depth       -A reproduces a POSIX ACL on every entry (gated on ACL
                   support + setfacl/getfacl).
  ownership-depth  --groupmap and --chown=:GROUP remap the group of every
                   entry (non-root, to a secondary group); -o/--usermap gated
                   on root.

All green on master and under --protocol=29/30.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Assert exactly which entries are/aren't transferred, deep in the tree:

  filter-depth      --exclude/--include precedence on files at every level, and
                    a -F per-directory .rsync-filter loaded from a deep dir that
                    applies to that subtree only (not above it).
  cvs-exclude       -C built-in cruft patterns (*.o, *~) at every level plus a
                    deep per-directory .cvsignore scoped to its subtree.
  size-filter       --max-size / --min-size select the right files all the way
                    down.
  files-from-depth  --files-from selects only the listed deep paths (implied
                    parents created); --from0 NUL-delimited; --exclude-from /
                    --include-from filter at depth.

(--existing / --ignore-existing are covered in delete-deep_test.py.)
Green on master and under --protocol=29/30.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drive a loopback daemon (secure stdio-pipe transport by default, also green
under --use-tcp) via the new write_daemon_conf helper and assert the behaviour
of the security-relevant rsyncd.conf parameters, transferring >=3-deep trees:

  daemon-access  path / read only / write only / list, incl. a deep sub-path
                 pull and that a list=no module is hidden yet usable by name.
  daemon-filter  daemon exclude hides matching files everywhere; incoming /
                 outgoing chmod rewrite modes of every transferred file.
  daemon-auth    auth users + secrets file accept the right password, reject a
                 wrong one and an unauthenticated request; strict modes rejects
                 a world-readable secrets file.
  daemon-exec    pre-/post-xfer exec run with RSYNC_MODULE_NAME /
                 RSYNC_EXIT_STATUS; a failing pre-xfer exec aborts the transfer
                 (marker files polled for, since post-xfer exec runs after the
                 client disconnects under TCP).
  daemon-munge   munge symlinks stores incoming links with the /rsyncd-munged/
                 prefix and strips it on the way out.
  daemon-refuse  refuse options: a named option, a wildcard, and the '* !a !v'
                 allow-list idiom.

Green on master under pipe and --use-tcp transports and under --protocol=29.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Breadth pass for options not yet exercised:

  output-options    output shape of --version/--help/-i/-n/--stats/
                    --out-format/--list-only/-q/--progress/-h/-8 (these control
                    output, not path handling, so they're checked for shape).
  compare           -c and -I catch a stealth change (same size+mtime, new
                    content) deep in the tree; --size-only skips a same-size
                    change; --modify-window absorbs a 1s mtime difference.
  compress-options  --compress-choice for every advertised compressor,
                    --compress-level, --skip-compress, --checksum-choice for
                    every advertised checksum, and --checksum-seed -- each a
                    clean byte-identical transfer at depth.

Green on master and under --protocol=29/30.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…oject#715 test

Add resolve_beneath_supported() to rsyncfns: it functionally probes whether the
rsync binary can follow an in-tree directory symlink under its secure resolver
(an initial transfer plus a delta update through a dir-symlink, the operation
issue RsyncProject#715 is about). This tracks the actual binary instead of a platform name.

Use it in symlink-dirlink-basis_test.py in place of the SunOS/OpenBSD/NetBSD/
Cygwin name check: it skips on those platforms too, and additionally on
Linux < 5.6, a seccomp-blocked openat2, and the new --disable-openat2 build,
where the portable O_NOFOLLOW fallback rejects the in-tree symlink.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two test-coverage build knobs (both behaviour-neutral by default):

  --enable-coverage  appends '--coverage -fprofile-update=atomic -O0' and adds
                     a 'make coverage' target (whole suite, run serially, then
                     gcovr HTML with branch + decision coverage). rsync forks
                     and its children exit without running the gcov atexit
                     flush -- the generator via its SIGUSR1 handler
                     (_exit_cleanup) and the receiver via the SIGUSR2 handler
                     -- so under GCOV_COVERAGE we call __gcov_dump() at both, or
                     receiver.c/generator.c record no coverage at all.

  --disable-openat2  gates the Linux openat2(RESOLVE_BENEATH) sites in syscall.c
                     on HAVE_OPENAT2 (defined by default), so disabling it forces
                     the portable per-component O_NOFOLLOW resolver to run as the
                     primary on ordinary Linux -- exercising and
                     coverage-counting that fallback tier without a pre-5.6
                     kernel. NOTE: coordinate with the parallel syscall.c
                     path-resolution restructure.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
COVERAGE.md is the living checklist mapping every CLI option (~142) and daemon
parameter (~54) to its test(s), with depth / cross-dir status and remaining
gaps, so the path-resolution restructure can see exactly what is guarded.

update_test.py closes two of the documented gaps: -u/--update (keep a newer
destination, update an older one) and --force (replace a non-empty destination
directory with a file), both at depth.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A delta (--no-whole-file) resume whose basis is an absolute --partial-dir
looped forever on exit code 23 ("failed verification -- update put into
partial-dir"), stranding the correct data in the partial-dir and never
populating the destination.

Cause: an absolute --partial-dir makes the basis path absolute, but the
receiver opened it with secure_relative_open(NULL, fnamecmp, ...), which by
design rejects an absolute relpath (EINVAL). The basis fd was then -1, so
receive_data() mapped no basis and (because the matched-block sum_update() is
guarded by "if (mapbuf)") computed the whole-file verification checksum over
the literal data only -> a spurious mismatch every run. (The data itself was
correct, since the in-place update leaves the matched basis bytes in place.)
Under a non-chroot daemon the in-place write went through the same call and
failed outright.

Fix: add secure_basis_open(), which treats an operator-trusted absolute basis
path as (trusted directory + confined leaf) -- the same way secure_relative_open
already trusts an absolute basedir while keeping O_NOFOLLOW on the leaf -- and
use it for both the basis read and the inplace-partial write. The strict
"reject absolute relpath" contract of secure_relative_open is left intact.

Defense-in-depth: receive_data() now treats a block-match token with no mapped
basis as a protocol inconsistency (it can only arise from a basis that the
generator opened but the receiver could not), failing cleanly instead of
silently dropping those bytes from the verify checksum or the output.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
partial_test.py sub-test 5 deterministically asserts a delta (--no-whole-file)
resume from an absolute, outside-tree --partial-dir reproduces the source and
consumes the basis -- the regression guard for the receiver fix. Sub-test 4
keeps asserting the cross-directory partial WRITE on interrupt. Drop the
--whole-file workaround and the 'broken on master' notes in the docstring and
COVERAGE.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
coverage-tcp reuses the coverage recipe with --use-tcp (daemon tests over a real
loopback rsyncd, which also runs the require_tcp-only tests) and a separate
report directory, via COVERAGE_RUNFLAGS / COVERAGE_DIR. Verified end to end:
pipe run reports 63.9% lines, the TCP run 64.5% (it exercises more code).

Also drop gcovr's --branches flag: it is deprecated in gcovr 8 and branch +
decision coverage still appear in --print-summary and the HTML without it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Builds with --enable-coverage and runs the suite under both transports
(make coverage, then make coverage-tcp). gcovr's line/branch/decision totals
are printed to the step log and also written to the GitHub step summary, so the
coverage numbers are visible directly in the CI output; the HTML reports are
uploaded as an artifact. make coverage exits with the suite's status, so a test
regression fails the job.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tridge tridge force-pushed the pr-test-coverage branch from f3e850e to cbf6394 Compare May 24, 2026 00:32
acls-depth skips where ACLs/setfacl are unavailable (macOS, Cygwin) like the
existing acls tests, and sparse skips on APFS (macOS), where a seek-written
hole isn't allocated sparsely. Add them to the per-platform RSYNC_EXPECT_SKIPPED
lists so the skip-set assertion stays accurate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tridge tridge force-pushed the pr-test-coverage branch from cbf6394 to 659047b Compare May 24, 2026 02:06
@tridge tridge merged commit f1d5a3c into RsyncProject:master May 24, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant